Skip to content

Public RenderOptions, FindReferencedTemplates and sandbox policy (0357 S2b, S2c) - #811

Merged
rmorozov merged 11 commits into
masterfrom
claude/project-thread-ab0irv
Nov 9, 2026
Merged

rmorozov merged 11 commits into
masterfrom
claude/project-thread-ab0irv

Conversation

@rmorozov

@rmorozov rmorozov commented Nov 9, 2026 •

Copy link
Copy Markdown
Member

Requested by Ruslan · project thread

Problem

Task 0357 moves the Python module from private engine seams to the public API before the G1 freeze (plan docs/design/0357-python-module-public-api.md, approved 2026-11-06). #800 landed the public host object (S1a, S2a). Three seams were still private:

This PR makes all three public: B3, B6 and B7/D6b. The module itself switches in S3.

Resulting behaviour

S2b (B3, B6)

  • jinja2::RenderOptions{ shared_ptr<const ValuesMap> globals; std::function<void()> poll; } and Template::RenderTo(out | sink, params, options).
    • The globals replace the environment's for this render and for the templates it includes, imports or extends.
    • The globals are shared rather than borrowed. A value a callable keeps stays valid after the render, as an environment global's does (LendGlobal).
    • The poll runs about every 1024 steps and may throw. The error then carries the exception in GetException().
    • A poll alone makes a render count its steps. A render that a polled one starts (a callable rendering another template) is polled too.
  • Template::FindReferencedTemplates() is now public. test/referenced_templates_test.cpp now builds in shared mode.
  • Bug found on the way: in a wide template every name came back nullopt. Fixed, with a test row.

S2c (B7, D6b)

  • New public jinja2::ISandboxPolicy in jinja2cpp/sandbox_policy.h, held by Settings::sandboxPolicy (compared by identity).
    • NativeAttribute(const Value&, string_view) -> optional<Value> is asked wherever the engine's policy was asked before.
    • Format(...) -> optional<Value> returns nullopt by default. The engine's own str.format then reads each field's attribute through NativeAttribute, as Jinja2's SandboxedFormatter does: '{0.__class__}|'.format(s) gives |.
  • The engine's interface is renamed IEngineSandboxPolicy, with one bridge (src/public_sandbox_policy.cpp), as for host objects.
  • Settings::immutableContainers is the public switch for the immutable sandbox.
  • New ErrorCode::SecurityError (21) replaces the extraParams[1] marker on an UndefinedError.
    • This changes the error code users see for the chat preset's refused list and dict methods: SecurityError, as in transformers' Jinja2.
  • UndefinedDescription::security makes and reads a refused attribute's undefined value.
  • 0427 is decided and marked done: a template keeps the policy it was loaded with, as it keeps its other settings. A test pins it.
  • Python module changes are minimal: poll on the new HostPollScope, the SecurityError code mapping, the rename. Its render entry is unchanged until S3.

Validation

  • Debug ctest: 8/8 passed.
  • Python module (Release, boost big ints, Python 3.12): pytest 1594 passed, 17 skipped. Upstream Jinja2 suite 596/856, unchanged.
  • scripts/preflight.sh --perf: all checks pass.
    • The instruction gate passes, and no row is above +0.3% locally (dec3a05). plain_text and substitute renders count the same as master; Load/plain_text is +9 instructions (+0.24%), 6 of them the Settings == comparison of the two new fields; Render/for_filter_if +0.30% (a step per item the filter skips).
    • The perf track held 859ce81 on a +14 per-render cost. Callgrind put all of it in template.cpp's copy of ~RenderContext, which called a robin_hood table destructor out of line once RenderCore<true> was compiled in the same file. TemplateImpl::RenderWithOptions now lives in src/template_render_options.cpp, and the TemplateParser constructor is forced inline.
  • Verifier, 1 round: ready, no blocking findings.
    • Debug and -Werror build with full ctest.
    • Clang ASan+UBSan: the full suite passed.
    • Shared build: the new suites pass and the new symbols are exported.
    • Public headers compile alone as C++17 with -Wpedantic.
    • Expectations checked against Jinja2 3.1.6 (SandboxedEnvironment, ImmutableSandboxedEnvironment, meta.find_referenced_templates).
    • Adversarial probes: kept globals, throwing and nested polls, a throwing policy, Format results that keep lent arguments.
    • Its one non-blocking note: formatMap was never set for a public policy. The header now says format_map comes from NativeAttribute, as the Python module does it.
  • Not run locally: MSVC, AppleClang, TSan (CI covers them).

Limits

  • A macro or caller reaches NativeAttribute as an empty value. S3 must keep the module's stand-in object for them.
  • TemplateEnvImpl::SetSandboxPolicy and SetDialect stay internal until the module switches (S3) and are deleted in S4.

Roles: verifier ×1 (1 round, ready). Red CI pushes: 1 (dec3a05: clang-tidy misc-include-cleaner in the new src/template_render_options.cpp; local preflight skipped the file because build/ had not been reconfigured).
Codex: 4 findings in 3 rounds, 4 fixed, 0 declined (e740760: a loop filter's skipped items and a backend-only limits scope now reach RenderOptions::poll, Render/for_filter_if +0.30%; da85d26: per-render globals are converted inside the render's limits scope and error boundary; 859ce81: a nested render under a backend-only scope is polled too). Both re-requests are used, so the last fix has no Codex re-review.


Generated by Claude Code

…lates (0357 S2b)

RenderTo(out|sink, params, RenderOptions) renders with the caller's own globals,
shared so that a value a callable keeps outlives the render, and a poll that may
throw to stop it. A poll alone makes the render count its steps; a render a polled
one starts is polled too. FindReferencedTemplates is public; in a wide template it
gave nullopt for every name and now converts them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AWNL7n7c5XUg2Xoo4ZRjLn
…; test includes

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AWNL7n7c5XUg2Xoo4ZRjLn
…:SecurityError (0357 S2c)

Settings::sandboxPolicy holds a public ISandboxPolicy, which one bridge turns into
the engine's policy (renamed IEngineSandboxPolicy). Its Format may decline: the
engine's own str.format then reads each field's attribute through the policy, as
Jinja2's SandboxedFormatter does. Settings::immutableContainers is the public
switch for the immutable sandbox. A refused attribute fails with the new
ErrorCode::SecurityError instead of an UndefinedError with a marker, and
UndefinedDescription::security makes and reads one. A template keeps the policy
it was loaded with (0427, done).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AWNL7n7c5XUg2Xoo4ZRjLn
…tidy fixes

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AWNL7n7c5XUg2Xoo4ZRjLn
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AWNL7n7c5XUg2Xoo4ZRjLn
@rmorozov rmorozov self-assigned this Nov 9, 2026

rmorozov commented Nov 9, 2026

Copy link
Copy Markdown
Member Author

@codex review
@codex security review


Generated by Claude Code

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 9, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-09T14:39:32.660466Z da85d26 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: af5416c0fc

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/template_impl.h Resolved
Comment thread src/template_impl.h Resolved
Codex on #811: a template loaded without limits had no step counting on
the items a for-loop's if-filter drops, and a big-integer backend alone
installed inactive limits, so RenderOptions::poll never ran. The filter
loop now counts a step per item, and a poll with no step limit set gets
the poll-only limits.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AWNL7n7c5XUg2Xoo4ZRjLn

rmorozov commented Nov 9, 2026

Copy link
Copy Markdown
Member Author

@codex review

Both P2 findings are fixed in e740760, with regression tests.


Generated by Claude Code

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e740760da5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/template_impl.h Resolved
Codex on #811: RenderOptions::globals were converted before RenderCore,
so a host BigInt past the 2^20-bit cap threw out of RenderTo instead of
returning ResourceLimitExceeded, and the conversion ran outside the
template's limits scope. RenderCore now converts them after installing
the limits, inside its boundary, as it does the params.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AWNL7n7c5XUg2Xoo4ZRjLn

rmorozov commented Nov 9, 2026

Copy link
Copy Markdown
Member Author

@codex review

The round-2 P2 (the conversion of per-render globals was outside the error boundary) is fixed in da85d26, with a regression test. This is the last re-request for this PR.


Generated by Claude Code

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: da85d26371

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/recursion_guard.cpp Resolved
Codex on #811: a render a polled one starts, on an environment with a
custom big-integer backend and no limits (non-Boost build), installed a
scope with inactive limits and was never polled. RenderLimitsScope now
counts steps for an inherited poll whenever its limits set none.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AWNL7n7c5XUg2Xoo4ZRjLn
@rmorozov
rmorozov marked this pull request as ready for review November 9, 2026 15:53
The perf track held #811 on a +14 instruction cost on every render.
Callgrind put it all in template.cpp's copy of ~RenderContext, the one
the linker keeps: with RenderCore<true> compiled in the same file, GCC
called a robin_hood table destructor out of line there. The RenderOptions
entry point (TemplateImpl::RenderWithOptions) now lives in
src/template_render_options.cpp, and plain_text and substitute renders
count the same as master. The TemplateParser constructor is forced
inline, since the move left Load/plain_text +0.51%; it is now +0.24%,
6 of the 9 instructions being the Settings comparison of the two new
fields.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AWNL7n7c5XUg2Xoo4ZRjLn
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AWNL7n7c5XUg2Xoo4ZRjLn
@rmorozov
rmorozov merged commit 1ed567f into master Nov 9, 2026
58 of 59 checks passed
rmorozov pushed a commit that referenced this pull request Nov 9, 2026
Marks 0427 done in the index.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DTQTriEsFrrBgcYNKDUUF4
rmorozov added a commit that referenced this pull request Nov 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants